refactor: preserve bound expression identity through scans - #9192
Conversation
Merging this PR will degrade performance by 4.4%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
c4540d3 to
0897fbf
Compare
d88a082 to
f5ad4f9
Compare
Polar Signals Profiling ResultsLatest Run
Previous Runs (1)
Powered by Polar Signals Cloud |
Benchmarks: String Encoding 📖vortex / vortex-file-compressed / ms (0.996x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Benchmarks: Random Access 📖Vortex (geomean): 0.994x ➖ vortex / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
vortex / parquet / ns (1.002x ➖, 0↑ 0↓)
vortex / lance / ns (0.991x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.992x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (1.023x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.006x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / duckdb / ns (1.007x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Clickbench on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 2↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.018x ➖, 2↑ 4↓)
duckdb / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / duckdb / ns (0.994x ➖, 1↑ 0↓)
File Size Changes (1 files changed, -0.0% overall, 0↑ 1↓)
Totals:
|
Benchmarks: PolarSignals Profiling 📖Vortex (geomean): 1.003x ➖ datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: FineWeb NVMe 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.989x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (1.004x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.009x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.017x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (1.028x ➖, 0↑ 2↓)
duckdb / parquet / ns (1.023x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: Statistical and Population Genetics 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (1.040x ➖, 0↑ 2↓)
duckdb / vortex-compact / ns (0.995x ➖, 1↑ 1↓)
duckdb / parquet / ns (1.005x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.997x ➖, 1↑ 1↓)
datafusion / vortex-compact / ns (0.997x ➖, 1↑ 1↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.993x ➖, 3↑ 4↓)
duckdb / vortex-compact / ns (1.009x ➖, 1↑ 6↓)
duckdb / parquet / ns (0.990x ➖, 8↑ 2↓)
duckdb / duckdb / ns (1.013x ➖, 1↑ 10↓)
No file size changes detected. |
Benchmarks: Compression 📖vortex / vortex-file-compressed / ns (0.988x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.992x ➖, 1↑ 0↓)
vortex / parquet / ns (1.000x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
|
Benchmarks: Vortex queries 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.968x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.992x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-H SF=10 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.108x ➖, 0↑ 3↓)
datafusion / vortex-compact / ns (0.943x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.940x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (1.019x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (0.985x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.017x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.976x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (0.960x ➖, 0↑ 3↓)
datafusion / parquet / ns (1.002x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.965x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (1.018x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.013x ➖, 0↑ 0↓)
|
Benchmarks: FineWeb S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.227x ➖, 0↑ 4↓)
datafusion / vortex-compact / ns (1.065x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.977x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.017x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.016x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.274x ➖, 0↑ 1↓)
|
Benchmarks: TPC-H SF=10 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.006x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.011x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / duckdb / ns (1.024x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Appian on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.014x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.988x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.989x ➖, 0↑ 0↓)
duckdb / duckdb / ns (1.008x ➖, 0↑ 0↓)
File Size Changes (1 files changed, -0.0% overall, 0↑ 1↓)
Totals:
|
Benchmarks: Clickbench Sorted on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.009x ➖, 1↑ 1↓)
datafusion / parquet / ns (1.023x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.996x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.021x ➖, 0↑ 0↓)
duckdb / duckdb / ns (0.959x ➖, 0↑ 0↓)
File Size Changes (201 files changed, +0.0% overall, 98↑ 103↓)
Totals:
|
| ext_storage(stat(geom.clone(), GeometryAabb.bind(EmptyOptions))) | ||
| } | ||
|
|
||
| fn stat(expr: BoundExpr, aggregate_fn: vortex_array::aggregate_fn::AggregateFnRef) -> BoundExpr { |
| session.register_rewrite(DynamicComparisonAllNonNanStatsRewrite); | ||
| } | ||
|
|
||
| fn binary(operator: Operator, lhs: BoundExpr, rhs: BoundExpr) -> BoundExpr { |
There was a problem hiding this comment.
shouldn't we have a centrialized place with these?
f5ad4f9 to
e3d332c
Compare
e3d332c to
627e28b
Compare
627e28b to
05ff6b6
Compare
05ff6b6 to
0f78dfc
Compare
0f78dfc to
cdb52ca
Compare
cdb52ca to
eda1715
Compare
d5e3946 to
9a180ab
Compare
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
9a180ab to
6f9301e
Compare
Summary
BoundExpressionWhy
Rebinding a structurally identical expression creates a new exact tree identity and defeats identity-keyed caches. Keeping one bound tree through scan planning and execution makes exact caching reliable.
Break
Use
Expression::bind(dtype)before passing a projection or filter to a scan plan.